Parse app config strictly by default #3629
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In #3376 we switched `UnmarshalYAMLStrict` to `UnmarshalYAMLNonStrict`. I'm not entirely sure if this was intentional, but we want this to be strict by default. Happy to discuss if the CLI needs non-strict anywhere and if so if we can make this configurable.
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
|
If we did it in that PR it was definitely intentional, that PR was unfortunately massive but I would not just revert this without understanding a bit more why or why not |
Yup that's why I opened the PR to discuss if this was intentional. I have poked around a little and can't see yet why we would do this, wondering if you can recall why we did this? |
|
I also cannot recall why. Maybe we could add a In other internal projects, we'd like to use strict unmarshal. It's a property we'd like to maintain to avoid config issues. |
saquibmian
changed the title
mfridman
approved these changes
Feb 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #3376 we switched
ReadConfigfromUnmarshalYAMLStricttoUnmarshalYAMLNonStrict. This was to enable a use case in an internal application. This PR switchesReadConfigback toUnmarshalYAMLStrictand addsReadConfigNonStrictto enable the non-strict use case. The internal application will document why it needs the non-strict property.I verified that nothing in the CLI needs non-strict parsing, everything is using the CLI's
NameContainerand readsbufapp.Config.